Skip to content

Adopt Semi.Avalonia and a FieldWorks design-token system - #1083

Open
johnml1135 wants to merge 2 commits into
mainfrom
semi-avalonia
Open

Adopt Semi.Avalonia and a FieldWorks design-token system#1083
johnml1135 wants to merge 2 commits into
mainfrom
semi-avalonia

Conversation

@johnml1135

@johnml1135 johnml1135 commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Replaces Avalonia's Fluent theme with Semi.Avalonia + Ursa and gives the Avalonia UI a real design-token system in place of colors/spacing scattered as literals — enforced by a new CI gate that fails the build on any hardcoded value in that surface. The Avalonia UI stays behind FW_AVALONIA, opt-in only; nothing here changes default FieldWorks behavior.

88 files is a lot for something with zero default-on behavior change — the honest reason: rebuilding the DataTree detail view on Ursa's layout primitive, and the token system it depends on, touch nearly everything already converted to Avalonia (9 dialogs + the detail-view foundation). The part actually worth scrutinizing is narrower than the diff: the token architecture and its enforcement gate. Most per-dialog changes are 1:1 literal→token swaps.

Where to look:

  • token-hygiene.ps1 is now a hard CI failure — whole-tree, no grandfathering, scoped to the Avalonia surface only. Why that scope, not global: docs/adr/0002.
  • FieldWorks tokens default to aliasing Semi's own semantic roles rather than inventing new colors, verified against the pinned Semi.Avalonia 11.3.14 source directly, not assumed: docs/adr/0001.
  • fwGroupBox (new titled-border primitive) is the one real visual change — applied only to the Options dialog, which had an actual reported defect; the other 8 were reviewed and left alone.
  • GenerateTokenKeys (new Build/Src/FwBuildTasks codegen) turns a stale/typo'd token key into a compile error — what makes "whole-tree, no exceptions" trustworthy rather than just strict-sounding.
  • DialogLayoutAssert gained 2 general checks (readable-font floor, group-box minimum gap) that run automatically on every dialog test already in the suite, not just new ones.

Deliberately not here: Dark/Compact/color-blind theming — the ThemeDictionaries structure supports all three later, none is built or visually verified now; Light is the only reviewed variant. No automated pixel-diff visual regression (why: docs/adr/0003) — verification is geometric assertions plus 9 committed baseline screenshots (Docs/migration/baseline-screenshots/). L10NSharp is a version bump only; live UI-language switching was investigated and explicitly not built (every language-change path in FieldWorks, old and new UI alike, already requires a restart).

Verification: Not stacked. Build: 0 errors. FwAvaloniaTests: 647 passed, 1 skipped (pre-existing). FwAvaloniaDialogsTests: 288 passed. Both hygiene gates clean. No native or installer files touched.


Reading this a year from now -- start here

This PR's working history (grilling sessions, adversarial reviews, live corrections) lived
in a long agent conversation, not in tree files — there was nothing to evict from the repo
because none of it was ever committed as scratch docs. What follows synthesizes that
conversation's decisions and evidence directly into this record.

The three ADRs (docs/adr/0001-0003) are the durable record of why; this section covers
the how it went, including the mistakes caught along the way.

The layer cake — token resolution, end to end

A view (C# in FwAvalonia/FwAvaloniaDialogs, or .axaml in the dialogs project) asks for
a value one of two ways:

  • XAML: {DynamicResource FwLabelBrush} or, for Semi's own roles now referenced
    directly, {StaticResource SemiColorDanger}.
  • C#: FwThemeResources.RequireBrush(GeneratedTokenKeys.FwLabelBrush) — a
    compile-time-checked constant, not a raw string, resolved at point-of-use via
    Application.Current.TryGetResource, never cached in a static field (Application.Current
    is null under beforefieldinit before the app starts).

The key resolves through merged Application.Resources: Src/Common/FwAvaloniaTheme's
FwColorTokens.axaml (Light/Dark ThemeDictionaries — shared brushes + the one
FwSurfaceFontSize) and DataTreeTokens.axaml (flat, non-themed DataTree layout
dimensions), both merged by FwAvaloniaApp/PreviewHostApp at Initialize(), plus
DialogTheme.axaml's own local Dialog* keys merged into that same dictionary and applied
per-dialog-body via DialogThemeBootstrap.Apply.

Underneath FieldWorks' tier sits Semi.Avalonia's own two-tier system: ~449 raw
color-ramp/spacing primitives (Layer 1, no meaning attached) and named semantic roles
(Layer 2: SemiColorText0-3, SemiColorBorder, SemiColorBackground0-4, a flat
spacing/radius/height scale) that alias them. FieldWorks' tier defaults to aliasing Layer 2
directly; a FieldWorks-owned value requires a written, checkable reason.

GenerateTokenKeys (a Build/Src/FwBuildTasks MSBuild Task, not a Roslyn generator — see
Decisions below) reads the token .axaml files' x:Keys at build time and emits
GeneratedTokenKeys.g.cs: the compile-time-checked constants above, plus baked Thickness
literal values (via Avalonia.Thickness.Parse) for the few spots
(CompactDialogStyles.cs/FwSurfaceStyles.cs) where Avalonia's compiled XAML rejects
x:Static, so a C# style builder can't read a token via {StaticResource} at all.

Decisions, and why

Alias Semi's semantic tier by default, not an independent FieldWorks palette. Semi
already ships primitive→semantic aliasing (the pattern every mature design system — Fluent
2, Carbon, Atlassian, Adobe Spectrum — uses); FieldWorks previously ignored it and picked
every color independently by eye from old WinForms screenshots. Verified empirically that
{StaticResource} reaches Semi's own Layer-2 keys fine from ordinary view XAML — the known
DynamicResource-only landmine on this branch is narrower than first assumed: it's specific
to DialogTheme.axaml's own Setters (grafted onto a view's .Styles at runtime), not
ThemeDictionaries crossing in general.

token-hygiene.ps1 is whole-tree and zero-grandfathering, deliberately unlike
comment-hygiene.ps1.
The scoped tree is new code with nothing to grandfather; current
design-token practice treats that as the correct case for full-strictness-from-day-one, the
same literature is equally clear it's the wrong call for retrofitting legacy code — which is
why the WinForms surface stays out of scope. Consequence accepted deliberately: since only
agents are required to run -TokenHygiene locally, one slipped-in violation on main fails
every unrelated PR touching the tree until fixed — no ratchet/baseline valve exists yet.

GenerateTokenKeys is a custom MSBuild Task, not a Roslyn source generator. Matches this
codebase's own precedent for "generate typed C# from a declarative source" — liblcm's
LcmGenerate — rather than introducing tooling nobody on this codebase has used yet.

Geometric layout assertions + reviewed screenshots, not automated pixel-diff. Real
current tooling for visual regression (Percy/Chromatic/Playwright) is a web/DOM-native
ecosystem with no mature managed equivalent for Avalonia/WPF; even mature web tooling needed
a dedicated AI-review layer to suppress anti-aliasing/font/DPI noise. A small, curated,
committed baseline set exists specifically so an "I looked, it's fine" claim survives past
the run that made it — previously all snapshots were ephemeral and gitignored.

Paths not taken
  • Live UI-language switching. The L10NSharp version bump was originally planned as a
    prerequisite for it. Investigated directly: every UI-language-change path in FieldWorks —
    WinForms and the existing Avalonia port alike — deliberately sets restartRequired = true
    rather than live-refreshing. Building live switching would be new, unrequested engineering
    inconsistent with the rest of the app, not a gap this branch needed to close.
  • A one-line suppression-comment mechanism for the hygiene gate, considered when
    discussing the gate's escape valve. Rejected once the actual existing exception (the
    x:Static compiled-XAML limitation) turned out to already be resolved better by
    GenerateTokenKeys generating the value outright, backed by
    DuplicateTokenPairConsistencyTests.cs as a regression guard — stronger than an unverified
    suppression comment would have been.
  • A generic "any two same-type siblings need a gap" rule for DialogLayoutAssert,
    rejected as too broad (would false-positive on deliberately tight pairs like a label over
    its field) in favor of a rule scoped specifically to fwGroupBox siblings.
What this does NOT authorize
  • Not a precedent for retrofitting token-hygiene.ps1-style enforcement onto the WinForms
    surface — that surface is out of scope by design (docs/adr/0002).
    Not a decision that Dark/Compact/color-blind theming is "done" — only that the
    ThemeDictionaries structure won't need a rearchitecture to add them later; none has been
    designed, built, or visually verified.
  • Not a claim that every FieldWorks-owned (non-aliased) color value has been re-verified
    pixel-for-pixel against Semi's real composited output — the 8 keys flagged in review were
    checked directly; the remaining KEEP-AS-NEW dimension tokens were not individually
    re-derived from Semi's spacing scale where no exact match existed.
Surprising findings
  • Two written justifications for keeping FieldWorks-owned colors instead of aliasing Semi
    were factually backwards: SemiColorBorder was described as "opaque, too heavy" for a 1px
    divider when the real value (verified against the pinned v11.3.14 tag's source) is
    8%-opacity and nearly invisible; Semi's Text0-3 were described as "resolving identically"
    when they're four distinct brushes with different baked opacities (0.8/0.62/0.35).
    Re-evaluating with the corrected facts didn't change any of the 8 affected keys' actual
    values — every one turned out to already be independently grounded in real legacy-WinForms
    pixel measurements — but the written reasoning in both FwColorTokens.axaml and
    docs/adr/0001 was wrong until corrected during review.
  • A branch-history squash (16 commits down to a handful, done via git commit-tree rather
    than an interactive rebase) initially produced a commit whose message described work
    actually contained in a different commit, because of a chronological mis-sequencing.
    Caught by an adversarial review that diffed each commit's actual content against its
    claimed content rather than trusting the message — re-sequenced and re-verified
    byte-identical to the pre-squash tree before proceeding.
Evidence
  • Build: .\build.ps1 -BuildTests -SkipNative -CommentHygiene -TokenHygiene — 0 errors, both
    hygiene gates clean (170 files scanned by token-hygiene.ps1, 0 violations).
  • Tests: FwAvaloniaTests 647 passed / 1 skipped (pre-existing, unrelated);
    FwAvaloniaDialogsTests 288 passed / 0 failed (284 baseline + 4 new fixture tests for the
    hardened DialogLayoutAssert checks).
  • gitlint --commits origin/main..HEAD clean.
  • Semi.Avalonia's actual resolved values (SemiColorBorder, SemiColorText0-3, the
    spacing/radius/height scale) were confirmed against the pinned 11.3.14 tag's real source
    (src/Semi.Avalonia/Tokens/Palette/Light.axaml) and, separately, a live headless resource
    walk under this repo's own TestAppBuilder/FwAvaloniaApp — not assumed from
    documentation or an earlier/different vendor version.
  • The two new DialogLayoutAssert checks are mutation-tested: a fixture with the real defect
    present (unreadable font size; a Margin="0" override defeating the themed group
    separation) fails, and the compliant case passes, for both.
  • No native, COM, or installer files are touched — confirmed via
    git diff --name-only origin/main...HEAD, entirely managed C#/XAML/PowerShell/docs.

This change is Reviewable

johnml1135 and others added 2 commits August 19, 2026 09:18
Replace the Avalonia Fluent theme with Semi.Avalonia + Ursa app-wide,
and rebuild the DataTree detail view on Ursa's Form/FormItem instead
of a hand-built Grid. The theme swap surfaced (and this fixes) real
layout regressions caught via actual screenshots: the pane not
filling its width, labels breaking mid-word, writing-system
abbreviations clipping, section headers centering instead of
left-aligning, duplicated header text. Field visibility on
collapse/expand now computes from the model (DetailVisibility)
instead of toggling realized controls.

Build a shared FieldWorks design-token system on top of that (new
Src/Common/FwAvaloniaTheme project, Light/Dark ThemeDictionaries),
replacing the color/spacing/font-size literals that used to be
scattered across FwAvalonia/FwAvaloniaDialogs. Every token defaults
to aliasing Semi's own semantic color/spacing roles (SemiColorText0,
SemiColorBorder, SemiColorBackground0, the Semi spacing/radius
scale, ...) rather than an independently-invented value; a
FieldWorks-owned value requires a written, checkable reason (see
FwColorTokens.axaml's comments) -- verified against the actual
pinned Semi.Avalonia 11.3.14 resources, not assumed.

Enforce this with Build/Agent/token-hygiene.ps1: unlike
comment-hygiene.ps1, it is not diff-scoped and has no grandfathering
-- every run scans the whole Avalonia surface (including the
Src/LexText/Src/xWorks trees future conversions will land in) and
fails on any hardcoded color/spacing literal. Wired into CI as a
hard failure. A new Build/Src/FwBuildTasks GenerateTokenKeys task
(following liblcm's LcmGenerate precedent, not a Roslyn source
generator) turns a typo'd/renamed token key into a build error
instead of a runtime throw, and bakes literal Thickness values for
the few spots where Avalonia's compiled-XAML x:Static limitation
previously forced a hand-duplicated literal.

Add a reusable fwGroupBox titled-border primitive (the WinForms
GroupBox analog) and apply it to the Options dialog, whose General/
Updates tabs previously applied one uniform spacing value to every
sibling alike -- an unrelated setting boundary read identically to a
label-to-its-field gap. Harden DialogLayoutAssert.AssertNoCrowding
with two general checks that run automatically on every dialog: a
readable-font-size floor, and a minimum gap between fwGroupBox
siblings. Commit a small, curated set of baseline screenshots
(Docs/migration/baseline-screenshots/) so a "this was reviewed and
looks right" claim has a surviving, checkable artifact instead of
living only in an ephemeral, gitignored capture.

Record the load-bearing decisions in docs/adr/0001-0003: aliasing
Semi's semantic tier by default, the whole-tree/no-grandfathering
hygiene gate scoped to the Avalonia surface only, and geometric
layout assertions plus reviewed screenshots instead of automated
pixel-diff visual regression testing.

Also upgrades L10NSharp 10.0.0-beta0004 to beta0014, a prerequisite
for future UI-language work that was investigated and explicitly not
built this branch: every UI-language-change path in FieldWorks,
WinForms and Avalonia alike, already deliberately requires a restart
rather than live-refreshing, so building live switching would be new
engineering inconsistent with the rest of the app, not a gap this
branch needed to close.

Independently rebuilt, retested, and hygiene-checked after every
commit throughout development, not just trusted from agent
self-reports -- caught and fixed a recurring CRLF/LF corruption bug,
an unauthorized subagent-forking-a-subagent race condition, a hygiene
gate that silently scanned zero files off-root, a test that didn't
test what it claimed to, a squash whose commit boundaries didn't
match its own messages, and (via independent adversarial review) a
hygiene gate that enforced a narrower slice than its commit message
claimed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Found during PR review-summary alignment: the doc still described a
now-removed architecture (three independent DialogFontSize copies
that "must stay equal") and the pre-token-system value (12px). The
actual, current state: one source (FwSurfaceFontSize in
FwColorTokens.axaml, value 11), resolved by all three consumers
directly, not hand-kept-equal.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

NUnit Tests

    1 files  ± 0      1 suites  ±0   11m 7s ⏱️ +15s
5 811 tests +27  5 730 ✅ +27  81 💤 ±0  0 ❌ ±0 
5 820 runs  +27  5 739 ✅ +27  81 💤 ±0  0 ❌ ±0 

Results for commit 41e46a1. ± Comparison against base commit 6c6f9eb.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.35158% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.06%. Comparing base (6c6f9eb) to head (41e46a1).

Files with missing lines Patch % Lines
Src/Common/FwAvalonia/Detail/DetailVisibility.cs 88.67% 3 Missing and 3 partials ⚠️
Src/Common/FwAvalonia/FwSemiLocale.cs 80.00% 2 Missing and 4 partials ⚠️
Src/Common/FwAvalonia/FwThemeResources.cs 66.66% 3 Missing and 1 partial ⚠️
Src/Common/FwAvalonia/FwAvaloniaDensity.cs 94.73% 3 Missing ⚠️
Src/Common/FwAvalonia/Detail/FwFieldControls.cs 95.65% 1 Missing and 1 partial ⚠️
Src/Common/FwAvalonia/Detail/DataTree.cs 98.98% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1083      +/-   ##
==========================================
- Coverage   38.07%   38.06%   -0.01%     
==========================================
  Files        1499     1503       +4     
  Lines      350141   350158      +17     
  Branches    40238    40243       +5     
==========================================
- Hits       133304   133303       -1     
- Misses     187558   187569      +11     
- Partials    29279    29286       +7     
Files with missing lines Coverage Δ
Src/Common/FwAvalonia/CompactDialogStyles.cs 100.00% <100.00%> (ø)
...c/Common/FwAvalonia/Detail/DetailRichTextChrome.cs 79.26% <100.00%> (ø)
Src/Common/FwAvalonia/Detail/FwOptionChooser.cs 84.08% <100.00%> (ø)
.../Common/FwAvalonia/Detail/FwStructuredTextField.cs 78.39% <100.00%> (ø)
Src/Common/FwAvalonia/Detail/HoverReveal.cs 91.35% <100.00%> (ø)
Src/Common/FwAvalonia/Detail/SliceFactory.cs 91.56% <100.00%> (+0.20%) ⬆️
Src/Common/FwAvalonia/FilterableDropdownSupport.cs 83.33% <100.00%> (ø)
Src/Common/FwAvalonia/FwAvaloniaApp.cs 78.57% <100.00%> (+28.57%) ⬆️
Src/Common/FwAvalonia/FwPosChooser.cs 87.82% <100.00%> (ø)
Src/Common/FwAvalonia/FwSemiDensity.cs 100.00% <100.00%> (ø)
... and 25 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jasonleenaylor jasonleenaylor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the most consequential change in the queue, so it got the most scrutiny — including
running your scanner against fixtures rather than reading it. The token architecture is
right, and I want it to land.

What is good, plainly. Aliasing Semi's semantic tier instead of inventing a palette is
the correct default and the ADR argues it well. GenerateTokenKeys turning a stale key into
a compile error is the right instinct and is the thing that makes "no exceptions" mean
something. DialogTheme.axaml + DialogThemeBootstrap are extended in place rather than
replaced. Deleting FwCheckBoxStyle/FwRadioButtonStyle (454 lines) instead of porting them
is right, because Semi exposes those sizes as overridable resources. New strings go through
.resx with no new L10NSharp. No test constructs a WinForms Form. The four rejected
alternatives in the body are each argued with evidence rather than taste, and the
live-UI-language-switching investigation that concluded "don't build it" is exactly the kind
of negative result worth writing down.

Timing. Before this lands, Hasso, Zach and Ken need to verify the timing — this is
foundational to everything downstream of it, and the sequencing is their call as much as the
content is mine.


1. Pin the theme variant

Nothing in the branch sets RequestedThemeVariant. A whole-branch search for
RequestedThemeVariant|ThemeVariant|ActualThemeVariant returns exactly one hit:
FwThemeResources.cs:49 reading app.ActualThemeVariant. Under Avalonia 11.3.17
(Directory.Packages.props:186), an unset RequestedThemeVariant means ActualThemeVariant
follows the OS app-theme setting, so a machine in dark mode resolves the Dark dictionary —
the one FwColorTokens.axaml:77-78 labels "unreviewed first-pass placeholders, not
design-approved", while the PR body says Light is the only reviewed variant.

The reason this is item 1 rather than a footnote: Dark is a complete 12-key palette, not a
stub. Every Light key has a counterpart, so it resolves silently instead of failing — Require
throws only on a missing key. An opt-in tester on a dark-mode machine sees an unreviewed UI
and has no way to know that is not the intended design, which makes their feedback misleading
as well.

Fix is one line at the top of FwAvaloniaApp.Initialize():

RequestedThemeVariant = ThemeVariant.Light;

Mirror it in PreviewHostApp so the two Initialize() bodies stay parallel as their comments
promise.

The test must assert RequestedThemeVariant, not ActualThemeVariant. Under headless the
latter reports Light regardless, so a test written the obvious way passes even unfixed.

2. The enforcement check stays blocking — but it has to be trustworthy first

I am keeping token-hygiene as a hard CI failure. The scope is narrow and real, and a check
that only ever runs advisory tends to stay advisory. But a blocking check must not misdescribe
itself, and this one does, in five places plus a bug.

2a. It can silently stop checking. token-hygiene.ps1:70-81 sets
FW_TOKEN_HYGIENE_REPORTED=1, and any later invocation in the same job exits 0 before
scanning
, regardless of -Advisory. Reorder the CI steps so any advisory run precedes the
build and the enforcing run becomes a no-op pass that reports success. This is the most
important fix in the item: an enforcement mechanism that can quietly disable itself is worse
than none, because it is trusted.

2b. Four documentation claims are false. build.ps1:128-129 ("CI reports violations as
warning annotations either way"), build.ps1:227-228 and test.ps1:129-130 ("in CI it only
annotates the pull request") — CI.yml:73 runs -TokenHygiene with an explicit
exit $LASTEXITCODE and no continue-on-error, and annotations are emitted only under
$Advisory (token-hygiene.ps1:62), so CI emits none and hard-fails. test.ps1:46-47 omits
that test.ps1:131 force-runs it whenever CI=true. TokenHygiene.psm1:282-285 says comments
are "masked only when the comment is the whole line's content", which is not what the code
does. AGENTS.md is the only one that matches reality.

2c. ADR 0001's enforcement paragraph describes a feature that does not exist. It states the
check "requires every value in the FieldWorks token files to be either a Semi alias or a
literal on a declaration line with a written justification comment — a literal with no comment
... fails". The entire implementation is TokenHygiene.psm1:360:
if ($line -match '\bx:Key\s*=') { continue }. No comment is required or checked. Someone
reading the ADR would believe the token dictionaries are policed when they are the least
policed files in scope.

2d. Close the worst holes. I ran the scanner against fixtures; every one of these passes
clean today:

<Border x:Key="X" Background="#FF0000" Margin="40" Padding="12,8"/>   (x:Key skips the line)
<Border Background="#00FF00"/> <!-- note -->                          (<!-- masks the line)
Opacity = 0.45,                                                       (not in the property list)
Padding = new Thickness(radio * 0.45)                                 (arithmetic evades)
Height = (double)18   /   MinWidth = 160.0m                           (cast/suffix evades)
var col = Avalonia.Media.Brushes.Red;                                 (lookbehind exempts it)
<Border Margin="8 4 8 4"/>                                            (numeric check is comma-only)
<Grid RowDefinitions="40,Auto" ColumnDefinitions="220,*"/>            (names not listed)

The x:Key and comment-masking ones matter most because they skip the entire line, and
TokenHygiene.Tests.ps1:204,209,323 encode that behaviour as intended. Narrowing both to the
matched declaration rather than the whole line, adding Opacity/Margin/Padding/
BorderThickness/CornerRadius to the C# property list, and accepting space-separated
Thickness values would close most of it. I am not asking for a perfect scanner — I am asking
that it not be defeated by appending a comment.

2e. "Whole-tree, no grandfathering" is 170 files out of 2309 under Src. That is a fine
scope; it is just not what the phrase says. Please state the real scope in the ADR and the body.

3. One-use tokens belong next to their view — the check already allows this

DialogTheme.axaml:97-116 hoists roughly twenty per-dialog constants into the shared dialog
dictionary, and the file's own comment apologises for it: "one-off, content-driven, but still
named for the token-hygiene gate."

That apology is unnecessary, because the check does not require it. I verified this by running
the scanner over a fixture: a value declared in a view's own <UserControl.Resources> is not
flagged (the declaration line contains x:Key=), and consuming it via {StaticResource} is
not flagged either. Only the literal usage trips. So a one-use value can live in the file that
uses it.

All six of the clearest offenders are consumed from exactly one .axaml and never from C#:

Key Only consumer
EntryGoAuxiliaryOptionsMaxHeight EntryGoDialogView.axaml:97
InsertEntryMatchesListMinHeight InsertEntryDlgView.axaml:114
LexOptionsComboMinWidth LexOptionsDlgView.axaml:59,134
MessageBoxIconGlyphSize MessageBoxView.axaml:42
AddNewSenseMinWidth AddNewSenseDlgView.axaml:8 (root element)
MessageBoxMinHeight MessageBoxView.axaml:7 (root element)

Please move them back and write the rule down somewhere durable: a value used by one view
lives in that view's own Resources; a value shared by two or more views, or consumed from
C#, goes in the shared dictionary.
The two set on a root element referencing its own
Resources are worth a quick confirm rather than an assumption.

That restores "shared" to meaning shared, and removes the only place where the check is
visibly distorting the design.

4. Account for the values that changed, and re-measure two colours

The body says fwGroupBox "is the one real visual change". Seven values moved:

Property main branch
LabelColumnWidth 96 150
WsAbbrevWidth 28 60
FieldSpacing 2 1
LabelBrush #6666B8 #696969
WsAbbrevBrush #4682B4 #404040
ValidationErrorBrush Firebrick #B22222 SemiColorDanger #F93920
PickerForegroundBrush #1A1A1A #1C1F23

plus three new: WsAbbrevMaxWidth 120, HotlinkBrush #0064FA, DisabledOptionBrush
#808080. FwColorTokenResolutionTests.cs:28,34 assert two of them, so they are intended —
but a 56% wider label column and a doubled writing-system gutter are not "no visual change",
and a reviewer reading the body would not go looking.

Please list each with a one-line reason under "Where to look". ValidationErrorBrush
especially: muted brick to vivid orange-red is a large perceptual jump, and the comment at
FwAvaloniaDensity.cs:205-208 says danger "is exactly what that role means, with no
FieldWorks-specific divergence to justify" — while FwColorTokens.axaml elsewhere declines
exactly this kind of drift ("so the measured value stays").

And the label colours need a real measurement, not a reworded claim. main
described #6666B8 as the "legacy label hue from the committed baseline pixels"; the branch
describes #696969 as "measured from the legacy baseline". Both claim the same source for
colours that are nowhere near each other (blue-violet versus grey; steel blue versus
near-black), FwColorTokens.axaml repeats the new claim, and ADR 0001 then cites those
comments as its provenance — so the evidence is circular and one half of it is false. You have
the baseline PNGs committed; please re-derive both values, state the method, and correct
whichever comment is wrong. For infrastructure like this the dictionary is the design
record, and the next person will treat whatever it says as measured fact.

5. Vendor-supplied strings must end up in Crowdin

What is here today is a net improvement, and worth saying so first. The only Ursa types used
anywhere are Form and FormItem (DataTree.cs), which render no text, and FwSemiLocale's
own docstring explains the real bug it prevents — both vendor themes reset to zh-CN on an
unrecognised locale, so without this class an Arabic user would get Chinese context menus.
Mapping the other locales to en-US is the right call.

The direction still needs to change. Semi now owns the wording of user-visible chrome — the
built-in TextBox context menu and validation furniture, in dialogs that do use text-bearing
controls (ChooserDialogView, CreateFeatureDialogView, EntryGoDialogView,
LexOptionsDlgView). A third-party vendor decides FieldWorks' vocabulary for six of the 29
locales in Installer.legacy.targets:527 and leaves the other 23 in English.

These strings need to come from a FieldWorks-owned .resx translated through Crowdin.
crowdin.json already globs Src/**/*.resx, so a resource file under Src/ flows to all
shipped locales with no config change — the work is enumerating Semi's localised keys and
overriding them. If that does not land in this PR, please record it as a Jira issue before
merge and reference it here, and label FwSemiLocale in its own summary as the interim
measure it is rather than the destination.

Either way, add a test pinning the locale lists against the shipped packages. They are
hand-transcribed from a version comment ("v11.3.14", "v1.15.1"), FwSemiLocale has no test
coverage at all, and drift in the wrong direction silently reintroduces the zh-CN default this
class exists to prevent.

6. Fix the layering inversion

FwAvalonia.csproj:83,93,97 feed ..\FwAvaloniaDialogs\DialogTheme.axaml into
GenerateFwTokenKeys, and FwAvaloniaDialogs.csproj:55 project-references FwAvalonia. So the
foundation's generated public API is determined by a file inside its own dependent. This
contradicts FwAvaloniaTheme.csproj:6-12, whose stated reason for existing is that the two
projects "share ONE token source without either depending on the other".

Concretely: GenerateTokenKeys.cs:114-116 throws InvalidDataException on an identifier
collision, so a Dialogs-only edit can fail the foundation's compile; and
FwAvaloniaApp.Initialize never merges DialogTheme.axaml, so the foundation publishes 23
Dialog* constants for keys its own app never registers.

The only consumer is CompactDialogStyles.cs (5 Dialog*Value uses), which is dialog styling
living in the foundation. Moving DialogTheme.axaml into FwAvaloniaTheme, or
CompactDialogStyles into Dialogs, removes the inversion entirely.

7. labelMaxWidth ignores the actual column width

DataTree.cs:439-440 computes labelMaxWidth from the token LabelColumnWidth (150) while
the real grid column comes from the host-supplied getLabelColumnWidth() (:92). This use is
new in this PR. Drag the splitter narrower than 150 and label wrapping stays capped at the
token, not the column. Either derive it from the same source the column uses, or say why the
token is correct here.

8. Pin the Ursa workarounds with tests

DataTree.cs fights Ursa's ControlTheme in three places — :107-110 (overriding
HorizontalAlignment=Left), :113-119 (a scoped style beating FormItem's Margin="0 8"),
:453-456 (local FontWeight.Normal beating a bold DynamicResource binding) — plus
:121-123/:152 depending on FormItem honouring only an absolute LabelWidth. All four
depend on undocumented internals of a 1.x dependency.

The only Ursa-aware test (DetailCustomFieldRenderingTests.cs:70) asserts none of them, and
VisualParityAndDensityTests.cs:141 asserts a constant rather than a rendered margin. An Ursa
upgrade that changes any of these regresses layout silently. Characterization tests asserting
the resolved alignment, margin and font weight would fail loudly instead, which is what you
want on a dependency you do not control.

9. Housekeeping

  • Stranded Fluent. FluentTheme is referenced by zero code on the branch but still
    package-referenced in FwAvalonia.csproj:37, FwAvaloniaTests.csproj:28,
    FwAvaloniaDialogs.csproj:35, FwAvaloniaDialogsTests.csproj:23,
    FwAvaloniaPreviewHost.csproj:23, plus the pin at Directory.Packages.props:192. Drop them —
    otherwise a Fluent DynamicResource can silently resolve again later.
  • PrivateAssets is backwards. Only FwAvalonia.csproj:39-41 marks Semi/Ursa
    PrivateAssets="all" — but FwAvalonia's runtime code needs Ursa, so the dependency does not
    flow and every consumer must redeclare it or hit a runtime FileNotFoundException. The other
    four projects omit it. Pick one contract deliberately.
  • VisualSnapshotTests.cs:105-114 (DetailEditView_AtWindowWidth_RendersCleanly) asserts
    nothing — it captures and returns, where the test above it calls
    DialogLayoutAssert.AssertNoCrowding. It cannot fail on a rendering defect and it inflates
    the 647-passed figure.
  • The word "gate". 22 newly-authored uses across AGENTS.md, build.ps1, test.ps1,
    TokenHygiene.psm1, token-hygiene.ps1 and the three ADRs, plus the filename
    0002-whole-tree-token-hygiene-gate.md. Please use "check"/"checker". This is all new text,
    so there is no existing-prose exemption.
  • Broken ADR links. The body's three links point at .../blob/semi-avalonia/docs/adr/...;
    the files are at Docs/adr/..., and GitHub blob paths are case-sensitive.
  • Docs/adr/ is a third location. Docs/architecture/ and openspec/ already exist for
    this, and AGENTS.md names Docs/lessons/README.md as the durable-lessons index. Three
    decisions of this weight should land in an existing system, or the new one should be
    explained.
  • Docs/adr/0002's central justification is an uncited appeal to "current design-token
    literature" (twice). For a decision that changes build policy for every contributor, name the
    source.
  • New group-box captions with no WinForms twinFwAvaloniaDialogsStrings.cs:41-45,56-57
    adds "Interface", "Startup", "Automatic Updates". The dialog-update skill requires approved
    divergences from the WinForms twin to be recorded in the conversion's Jira issue; no LT-
    reference appears in the branch.

Questions, not change requests

  • Brush identity. These properties now return the shared, mutable SolidColorBrush
    from the dictionary, where main returned immutable Brushes.* singletons. You reason about
    this for TransparentBrush (FwAvaloniaDensity.cs:230-236) but not for SliceRuleBrush,
    SectionRuleBrush or PickerBorderBrush. Is anything relying on reference equality or
    mutating one?
  • Does FwBuildTasks need Avalonia? FwBuildTasks.csproj:26-27 takes the dependency solely
    for Thickness.Parse (GenerateTokenKeys.cs:127-133,180-188), which puts Avalonia.Base and
    its closure into BuildTools/ for every build in the repo. Parsing one to four
    comma-or-space-separated doubles by hand is a few lines.
  • Absolute paths in the generated file. GenerateTokenKeys.cs:141,146 writes the raw input
    paths into the header, and those come from $(MSBuildProjectDirectory)\..\..., so
    GeneratedTokenKeys.g.cs differs byte-for-byte between machines. It is gitignored so the
    impact is small, but a repo-relative path is a one-liner.
  • L10NSharp jumps ten betas (SilVersions.props:22, beta0004 to beta0014) inside a theming
    PR, with the body noting only that live language switching was investigated and dropped. What
    else moved in those ten?
  • FwSemiDensity.cs:35-39 computes radio * 0.45 while its own adjacent comment cites
    Semi's ratio as "~0.375". Which is right?

Noted, not asked for

  • High contrast. Not a regression — 150 WinForms files still use SystemColors and track
    the OS scheme. But Avalonia maps a high-contrast scheme onto Light or Dark by whether its name
    contains "White", so a High Contrast Black user lands in that unreviewed Dark palette. Pinning
    Light (item 1) does not fix high contrast; it makes the failure consistent and reviewed rather
    than arbitrary. Real support is out of scope here and worth its own ticket.
  • Dependency licences. Semi and Ursa are both MIT and compatible, and this PR records
    neither — but the repo has no NuGet-licence convention at all (Docs/architecture/dependencies.md
    is about repo dependencies, and no NOTICE file is tracked), so this PR is consistent with
    existing practice. Raising it as a gap for someone to close, not as something you introduced.

Comments

FwAvaloniaDensity.cs appends the same sentence — "Resolved from the shared FwAvaloniaTheme
token dictionary (DataTree.X) at point-of-use, after the Application has started" — to roughly
25 properties, beside code that already reads
FwThemeResources.RequireDouble(GeneratedTokenKeys.DataTree_X). It restates the code, it is
HOW rather than WHAT, it names the collaborator, and it duplicates the type-level precondition
in every member. One statement on the type, none on the members.

Also: cross-file pointers (FwColorTokens.axaml:11-12, TokenHygiene.psm1:15-16); absence
narration (FwColorTokens.axaml:10-12, FwSemiDensity.cs:10-15, LexOptionsDlgView.axaml:34-38
and :115-119); DialogThemeBootstrap.cs:65-68, a four-line comment explaining why no style is
added, followed by nothing — delete it; a finding reference at DataTree.cs:121 ("Viewing
parity (11.15)"); and consumer instructions in FwThemeResources.cs:12-25. FwThemeResources
should carry an <exception> tag for the InvalidOperationException its whole design rests on
— which is also the one path Codecov shows uncovered.

The per-key rationale comments in DataTreeTokens.axaml and FwColorTokens.axaml are the good
ones: real WHY, one or two sentences, correctly using single hyphens inside XML comments. Keep
those.

Finally, dozens of the rewrapped comments break mid-phrase leaving orphan one-word lines
(FwAvaloniaApp.cs:17-18, FwThemeResources.cs:14-19, FwSemiDensity.cs:26-29,
DialogTheme.axaml:71-74, DialogLayoutAssert.cs:21-23, which also loses its bullet
indentation). Not a rule violation, but it reads as machine-mangled throughout.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants